chore(deps): update rules_cc and protobuf versions#3367
chore(deps): update rules_cc and protobuf versions#3367mai93 wants to merge 12 commits intobazel-contrib:mainfrom
Conversation
rickeylev
left a comment
rickeylev
left a comment
There was a problem hiding this comment.
I pushed an update to the changelog.
Please update the PR description to explain why these bumps were necessary.
Mostly LGTM, sans the question and need to create deps.bzl function
| py_repositories() | ||
|
|
||
| # Needed for rules_cc 0.2.10 | ||
| load("@bazel_features//:deps.bzl", "bazel_features_deps") |
There was a problem hiding this comment.
Instead of copy/pasting this same setup to every downstream workspace, create a second setup function and load and call that.
python/repositories_deps.bzl
which does the 4 lines
There was a problem hiding this comment.
I tried having a file load_cc_deps.bzl that looks like this:
load("@bazel_features//:deps.bzl", "bazel_features_deps")
load("@rules_cc//cc:extensions.bzl", "compatibility_proxy_repo")
def add_cc_deps():
bazel_features_deps()
compatibility_proxy_repo()
but it did not work:
ERROR: Failed to load Starlark extension '@@bazel_features_version//:version.bzl'.
Cycle in the workspace file detected. This indicates that a repository is used prior to being defined.
because bazel_features_deps() call needs to happen before the load("@rules_cc//cc:extensions.bzl", "compatibility_proxy_repo") and we cannot have load statements in starlark functions, or what you're suggesting is different?
| - "--noenable_bzlmod" | ||
| - "--build_tag_filters=-integration-test" | ||
| # recent abseil-cpp requires C++17 which is not used by default in Bazel 7 | ||
| - "--cxxopt=-std=c++17" |
There was a problem hiding this comment.
Can you explain a bit more about this? I'm guessing this is coming from protobuf?
There was a problem hiding this comment.
yes, for protobuf 33.0 (and its dependency abseil-cpp) C++17 is the minimum supported version (https://github.com/google/oss-policies-info/blob/main/foundational-cxx-support-matrix.md)
I'm trying to add these flags to make it work with Bazel 7 but it is not straightforward as it is still failing for tests on windows (https://buildkite.com/bazel/rules-python-python/builds/13428#0199f84a-0a7c-4ed7-8b30-6ab8ef898f71) where -std=c++17 is not recognized. Do you have any suggestions for that?
|
Playing devil's advocate. This will be a breaking change for bazel 7 users who rely on protobuf. Should we cut a rules_python 2 where we remove protobuf dep instead? We can still merge the PR, but we could do a lightweight version 2 release with cleaning a few things up afterwards. |
See jvolkman/rules_pycross#209 The old protobuf version still needs Python 3.8, so attempting to load it fails. I see there is already a PR here (bazel-contrib#3367) but this is a much smaller change for an isolated issue.
|
Bump on this, this is now also necessary since the 3.8 support drop, the old version needs 3.8 still. I opened another PR which is failing for some reason, but if this one fixes that issue I'm happy to close mine. |
4 participants
These updates are mainly needed for compatibility with bazel 9:
rules_ccto 0.2.10 to fix the cyclic dependency error while testing with bazel 9 pre-releaseprotobufto 33.0 which has load statements forrules_ccneeded for bazel 9